Skip to content

Choropleth Counties #929

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 39 commits into from
Feb 16, 2018
Merged

Choropleth Counties #929

merged 39 commits into from
Feb 16, 2018

Conversation

Kully
Copy link
Contributor

@Kully Kully commented Jan 29, 2018

Re: #897





Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there are a lot of extra newlines added at the bottom of this file

@@ -1 +1 @@
__version__ = '2.3.0'
__version__ = '2.3.1'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this should be 2.4.0 https://semver.org/

@@ -11,6 +11,7 @@

import numpy as np
from scipy.spatial import Delaunay
import os
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

did os need to be imported for the added choropleth test? It doesn't look like it below.

fig['layout']['yaxis']['range'][0] = center[1] - new_height * 0.5
fig['layout']['yaxis']['range'][1] = center[1] + new_height * 0.5

return fig
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

newline at the end of file recommended

@jackparmer
Copy link
Contributor

Do we need this xml file?
https://github.com/plotly/plotly.py/pull/929/files#diff-b69057916476b49938dab4751dc2413e
Are there any other files we should remove that we don't need?

@jackparmer
Copy link
Contributor

Just to confirm, the current version of this FigureFactory in this PR generates these plots?
https://plot.ly/~AdamKulidjian/6680

@@ -557,7 +557,7 @@ def create_choropleth(fips, values, scope=['usa'], binning_endpoints=None,
"""
# ensure optional modules imported
if not gp or not shapefile or not shapely:
raise ImportError("geopandas, shapefile and shapely must be "
raise ImportError("geopandas, pyshp and shapely must be "
"installed for this figure factory")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add the pip statements here? For example, "Try pip install geopandas and pip install pyshp in your terminal."

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good idea. we can add it to the docs as well to remind in two places

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to confirm

confirmed

@jackparmer
Copy link
Contributor

There's a mismatch between this notebook and the plots. I think you updated the plots, but didn't upload the new notebook: https://plot.ly/~AdamKulidjian/6680 Mind uploading the latest?

@jackparmer
Copy link
Contributor

💃 After considering the comments above. Great work on this one. By far the easiest way to make interactive choropleth maps in Python to the best of my knowledge. 🥂

image

@Kully
Copy link
Contributor Author

Kully commented Feb 16, 2018

Mind uploading the latest?

Here's the latest notebook: https://plot.ly/~AdamKulidjian/6681

@Kully Kully merged commit 1fb2547 into master Feb 16, 2018
@Kully Kully deleted the zipcode-choro branch February 16, 2018 18:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants